Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PLT-3505 small fixes to the isabelle specification #168

Merged
merged 24 commits into from
Apr 14, 2023

Conversation

hrajchert
Copy link
Collaborator

@hrajchert hrajchert commented Mar 15, 2023

TODO: This is marked as a draft until PR 161 has been merged.

This PR adds a changelog starting from the audit and addresses several small findings of the first milestone report.

Screen Shot 2023-03-15 at 12 04 28

Screen Shot 2023-03-15 at 17 13 51

Screen Shot 2023-03-15 at 17 31 42

Screen Shot 2023-03-15 at 17 37 55

Screen Shot 2023-03-15 at 17 45 03

Screen Shot 2023-03-15 at 18 09 29

Screen Shot 2023-03-15 at 18 26 08

Screen Shot 2023-03-15 at 18 33 53

Screen Shot 2023-03-15 at 18 39 11

Screen Shot 2023-03-16 at 10 39 50

Screen Shot 2023-03-16 at 10 49 03

Screen Shot 2023-03-16 at 16 47 48

Screen Shot 2023-03-16 at 17 00 44

Screen Shot 2023-03-16 at 16 53 05

Screen Shot 2023-03-16 at 17 08 01

Screen Shot 2023-03-16 at 17 18 07

Screen Shot 2023-03-16 at 17 35 51

Screen Shot 2023-03-16 at 17 53 54

Screen Shot 2023-03-16 at 18 01 01

Screen Shot 2023-03-17 at 10 45 26

Screen Shot 2023-03-17 at 10 45 35

Screen Shot 2023-03-17 at 10 58 51

Screen Shot 2023-03-17 at 11 45 00

@hrajchert hrajchert marked this pull request as draft March 15, 2023 15:03
@palas palas force-pushed the hrajchert/list-accounts branch 2 times, most recently from a66ce09 to 40274af Compare March 22, 2023 20:54
Base automatically changed from hrajchert/list-accounts to master March 23, 2023 14:05
@hrajchert hrajchert force-pushed the hrajchert/scp-3505-small-fixes branch 2 times, most recently from 81e76e4 to 2f1fcc7 Compare March 23, 2023 14:08
@hrajchert hrajchert marked this pull request as ready for review March 23, 2023 14:14
Copy link
Member

@yveshauser yveshauser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks. Just some small corrections, resp. comments

@@ -165,7 +170,7 @@ text \<open>@{thm evalValue_MulValue}\<close>

subsubsection \<open>Division\<close>

text \<open>Division is a special case because we only evaluate to natural numbers:
text \<open>Division is a special case because we only evaluate to integers:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
text \<open>Division is a special case because we only evaluate to integers:
text \<open>Integer division is implemented as follows:

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now obsolete as I am removing Core.thy and rephrasing the whole thing in the refactor Semantic file.

The code/PDF will look like following:
Screen Shot 2023-04-07 at 11 35 10

any suggestion will be better made at that PR once it is ready.

(* TODO: This will become clearer after refactoring the semantics as literal programming PLT-3761 *)
text \<open>The @{const giveMoney} function is used in @{const reduceContractStep} to execute a Payment.
It takes as arguments the Party to remove funds from, the Payee to pay to, the amount and token to pay
and the state accounts. It returns the Payment as a Reduce effect and the new state accounts.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not necessary to mention that the accounts are part of the state here

Suggested change
and the state accounts. It returns the Payment as a Reduce effect and the new state accounts.
and the accounts. It returns the Payment as a Reduce effect and the new accounts.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar note, removing Core.thy. For the moment I'm not even mentioning even the Accounts, moreover, I just give a vague description and a comment that we need to rethink that function

Screen Shot 2023-04-07 at 11 40 41

isabelle/Doc/Specification/Core.thy Show resolved Hide resolved
isabelle/Core/Semantics.thy Show resolved Hide resolved
@@ -1234,5 +1226,5 @@ lemma inputToTransactions_isSingleInput : "isSingleInput (inputsToTransactions s
lemma traceListToSingleInput_isSingleInput2 : "isSingleInput (traceListToSingleInput t)"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

traceListToSingleInput_isSingleInput2 is unused in the rest of the code base as well

@hrajchert hrajchert force-pushed the hrajchert/scp-3505-small-fixes branch from 5694151 to 97fac7d Compare April 7, 2023 14:51
@hrajchert hrajchert merged commit 842648f into master Apr 14, 2023
@hrajchert hrajchert deleted the hrajchert/scp-3505-small-fixes branch April 14, 2023 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants